Thanks for the PR! Something for us to consider:

Every time I see a PR here I get a feeling of dread: we have no test coverage and so I have very little visibility or intuition of the impact that these changes make. What kind of infrastructure would we have to add for us to have unit test coverage for these kinds of changes?

Part of the reason this is still a draft is that I'd like to add some tests here. They'll be limited in scope though.

Here are the levels of tests we have in our R stack, by ascending level of scope:

• Internal LSP tests, these are easiest to write.
• Protocol-level tests for LSP with a custom client.
• Extension tests. With these you can test effects of vscode commands on e.g. text in an editor. These commands may delegate to the LSP. Example: https://github.com/posit-dev/positron/blob/main/extensions/positron-r/src/test/indentation.test.ts
• UI tests in the Positron suite: https://github.com/posit-dev/positron/tree/main/test/e2e/tests/quarto

We spend most of our time and energy on internal LSP tests.

@lionel- we now have extension tests! The tests are focused on roundtripping right now, but they do already involve executing commands. Do you think an extension test (a snapshot test?) that opens our test qmd files, executes the format command, and checks the contents of the file would provide enough coverage for this PR?

I'd like to get this PR in, so I'm happy to write the tests. Please advise!

@vezwork It turns out that @lionel- is OOO all this week. I think an extension test that formats and checks the contents is a great way to go, and if you are wanting to work on this soon-ish (before conf) I am happy to review if you can contribute a test or two!

I pulled down this PR, rebased on main, and followed the reproduction steps in #745 (which this PR should address). I ran into a new error that prevented any formatting from occuring:

I was able to reproduce #745 in Positron on the main branch of the extension, so this error is confirmed to be caused by this PR.

I have some local changes where I'm able to load the minimal repro qmd specified in #745, set the editor's cursor to inside a code cell, and do vscode.commands.executeCommand("quarto.formatCell"); but no formatting happens and a toast pops up in the bottom-right of the test VSCode that says

Editor selection is not within a code cell that supports formatting.

...but the cursor is definitely within a cell. I suspect that the Quarto extension is toasting this because there is no R formatter available.

The test VSCode instance has 0 extensions installed: I know from looking under the extensions tab while the tests are running. That means there's no R extension. That means there's no R formatter available!

It is confusing to me that there are 0 extensions installed because the vscode testing guide says:

When you debug an extension test in VS Code, VS Code uses the globally installed instance of VS Code and will load all installed extensions.

but that does not seem to be the case for our extension tests. For one other thing, the test instance of VSCode always has an Abyss theme which is not my local VSCode theme, I don't know why. @juliasilge do you know why our test VSCode instance does not have any extensions installed?

Interesting, no, I don't know why. I just tried it myself and I do in fact also see no extensions installed in the test runner VS Code that gets downloaded when you start such tests:

I wonder if this literally means "debug" rather than run:

When you debug an extension test in VS Code

Like when you do Test: Debug All Tests rather than run from the CLI, for example.

You might try adding the air and/or ruff extensions as extension dependencies just to see if you can get it installed in the test runner that way:

  "extensionDependencies": [
    "posit.air-vscode",
    "charliermarsh.ruff"
  ]

Not that we want to really depend on these, but to explore what our options are.

Another thought I had is that we might want to mock out a formatter to use in the tests, or maybe two (one that adds a final newline and one that does not).